Fix Android Fabric Native Animated null prop crash by jingjing2222 · Pull Request #56913 · facebook/react-native

jingjing2222 · 2026-05-21T02:27:52Z

Summary

This fixes an Android Fabric crash in the overrideBySynchronousMountPropsAtMountingAndroid path when Native Animated has stored a synchronous transform / opacity override and a later regular Fabric commit contains null for the same prop.

As-is	To-be

crashed.gif	patched.gif

Native Animated can synchronously update transform / opacity on the UI thread. A regular Fabric commit for the same view may arrive later with stale props, including transform: null or opacity: null. Those regular Fabric null values should not clear the stored synchronous override, because the override may still represent the fresher Native Animated value.

Before this patch, the override merge path assumed that the incoming Fabric prop had the same non-null type as the stored override. For example, a stored transform override is an array, but a regular Fabric update may contain transform: null. That mismatch could crash in SurfaceMountingManager.overridePropsReadableMap.

This patch:

Allows regular Fabric null updates to be overridden by stored synchronous transform / opacity values instead of crashing.
Keeps regular Fabric transform: null / opacity: null from clearing the stored synchronous override.
Scopes synchronous null clearing to transform / opacity only.
Leaves null values for other props untouched, since other view prop nulls may be meaningful.

This PR intentionally does not change Java Native Animated restoreDefaultValues() behavior. That restore implementation can be handled separately because changing it may affect existing apps.

Repro repository:
https://github.com/jingjing2222/react-native-fabric-transform-repro

Why this is correct

A regular Fabric null update is not enough to prove user intent to clear the native animated value, because it can be the stale commit produced after Native Animated already wrote a fresher value on the UI thread. In that case, the stored synchronous override should continue to win.

If the synchronous Native Animated path explicitly sends transform: null or opacity: null, this patch treats that as a clear signal for that stored override key. The clear behavior is intentionally scoped to the props currently stored by this override path.

regular Fabric null: possible stale commit -> stored synchronous override wins
synchronous transform / opacity null: explicit clear -> stored override key is removed
other null props: left untouched

Changelog:

[ANDROID] [FIXED] - Fix Android Fabric Native Animated crash when regular Fabric null props collide with stored synchronous transform or opacity overrides.

Test Plan:

Added regression coverage for stale regular Fabric null commits and scoped synchronous null clears:

updateProps_withNullOpacity_keepsStoredSynchronousProp
updateProps_withNullTransform_keepsStoredSynchronousProp
updatePropsSynchronously_withNullOpacity_removesStoredSynchronousProp
updatePropsSynchronously_withNullTransform_removesStoredSynchronousProp

Ran:

./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.fabric.SurfaceMountingManagerSynchronousMountPropsTest -Preact.internal.useHermesStable=true

Result:

BUILD SUCCESSFUL

Ran:

./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.animated.NativeAnimatedNodeTraversalTest -Preact.internal.useHermesStable=true

Result:

BUILD SUCCESSFUL

Ran:

./gradlew :packages:react-native:ReactAndroid:ktfmtCheck -Preact.internal.useHermesStable=true

Result:

BUILD SUCCESSFUL

meta-codesync · 2026-05-21T13:09:35Z

@zeyap has imported this pull request. If you are a Meta employee, you can view this in D105955122.

zeyap · 2026-05-21T13:24:52Z

+          }
+
          if (propKey == PROP_TRANSFORM) {
            assert(outputReadableMap.getType(propKey) == ReadableType.Array && propValue is List<*>)


In fact if Fabric commit includes a null value for transform that means you're doing a setState on other prop on this view, override on transform shouldn't be cleared because of that

I'd recommend not doing the change on lines 1332-1343 and let this check allow null from outputReadableMap

val outputType = outputReadableMap.getType(propKey) assert( (outputType == ReadableType.Array || outputType == ReadableType.Null) && propValue is List<*> )

zeyap · 2026-05-21T13:25:14Z

Also here

val outputType = outputReadableMap.getType(propKey) assert( (outputType == ReadableType.Number || outputType == ReadableType.Null) && propValue is Number )

could you try and see if animation behavior is still expected?

I'm trying 👍

zeyap · 2026-05-21T13:26:58Z

+      if (directPropsMap.isEmpty()) {
+        tagToSynchronousMountProps.remove(reactTag)
+      }


this is not right, tagToSynchronousMountProps will be cleared after 1 regular Fabric commit

zeyap · 2026-05-21T13:47:44Z

to clarify

storeSynchronousMountPropsOverride — records the override values previously applied synchronously by Native Animated
updatePropsSynchronously — called by Native Animated, applies props directly, skips the override merge (since this is the source)
updateProps — called by React commit path, applies stored overrides on top of (potentially stale) Fabric values

jingjing2222 · 2026-05-21T14:33:50Z

@zeyap

I tried this direction and I think the behavior matches what you described.

Normal Fabric updateProps(transform: null) should not clear the stored synchronous override, since that commit can still be stale relative to Native Animated. With only that change, though, the repro does not reset after clearing transform, because the stored override keeps overwriting the incoming null.

So I moved the reset to the synchronous path instead restoreDefaultValues sends transform: null, and storeSynchronousMountPropsOverride removes the stored key when the synchronous props contain null.

This seems closer to the iOS behavior too: regular Fabric commits do not overwrite animated-managed props, but the synchronous animated path can still reset them.

zeyap · 2026-05-21T15:13:55Z

+      val synchronousMountProps = tagToSynchronousMountProps[reactTag] ?: mutableMapOf()
+      removeNullPropsFromPropsReadableMap(props, synchronousMountProps)
+      synchronousMountProps.putAll(propsMap)
+      if (synchronousMountProps.isEmpty()) {
+        tagToSynchronousMountProps.remove(reactTag)
      } else {
-        synchronousMountProps = propsMap
+        tagToSynchronousMountProps[reactTag] = synchronousMountProps
      }


I would recommend keeping this function storeSynchronousMountPropsOverride as is.

you're now removing reactTag from tagToSynchronousMountProps if tagToSynchronousMountProps[reactTag] is empty, I think it's okay to keep but may be no-op

adding removeNullPropsFromPropsReadableMap might cause issue, some prop's (not transform) null value could be actually meaningful. updateProps can take all types of viewprops so i'd be cautious

hm, i guess the issue is if we dont clear the null props, restore doesnt really clear override as you expected?
if you see this issue, for the short term we can do this: if opacity and transform receive null prop, clear them; for other props we keep null as is, there probably should be an API to explicitly clear a prop from override map

Yes, exactly. If restoreDefaultValues() sends transform: null but the stored synchronous override is not cleared, the restore only updates the view once. The old override can still remain in tagToSynchronousMountProps and win again on a later regular Fabric commit.

That is why I originally handled both pieces together.

restoreDefaultValues() sends a synchronous null update for the animated props it owns.

The synchronous override store treats that null as a clear signal and removes the stored override key.

I agree that generic null removal is too broad, though. We can scope the clear logic only to the props that this override map actually stores today (transform and opacity), instead of removing every null prop key.

Given that the restore behavior depends on clearing the stored override, do you think it would be better to keep both changes in this PR, with the null-clear logic scoped to transform / opacity?

If you prefer keeping this PR focused only on the regular Fabric updateProps null handling, I can split the restore behavior into a follow-up PR now.

In that case, I’ll remove the restoreDefaultValues() change and the null-clear logic from this PR, and keep this one limited to allowing stored synchronous transform / opacity overrides to win over regular Fabric null updates.

That makes sense. I’ll go with the short-term scoped fix here.

I’ll keep the null-clear behavior limited to the props that are currently stored in this override map: transform and opacity. So synchronous transform: null / opacity: null can clear the stored override for restore, while null values for other props are left untouched.

I agree that a more explicit API for clearing a prop from the override map would be cleaner longer term. I’ll update the patch in this direction and push again.

Yes let's scope clear logic to transform and opacity

Let's move restoreDefaultValues to another change because I'm concerned that restore was no-op, so adding things here will regress existing apps.
I realize restoreDefaultValues is not fired at reset(), but you mentioned resetting actually restores the value

react-native/packages/react-native/src/private/animated/createAnimatedPropsHook.js

Line 291 in 5c937d7

node.__restoreDefaultValues();

zeyap · 2026-05-21T15:14:52Z

    connectedViewTag = -1
  }

  fun restoreDefaultValues() {


thanks for adding this I think java NativeAnimated misses impl for restore
but could you put it in a separate PR?

zeyap · 2026-05-21T15:15:31Z

+            val outputType = outputReadableMap.getType(propKey)
+            assert(
+                (outputType == ReadableType.Array || outputType == ReadableType.Null) &&
+                    propValue is List<*>)


jingjing2222 · 2026-05-21T16:28:46Z

@zeyap

Thanks for the helpful review 🙇‍♂️🙇‍♂️🙇‍♂️

I updated the PR based on your feedback and also refreshed the repro evidence.

One thing I want to confirm: from the product behavior perspective, the expected result after clearing the animated transform is that the view returns to its original position. With restoreDefaultValues() kept out of this PR, this change fixes the crash but does not complete that reset behavior by itself.

Do you already have a plan for handling the restore behavior separately, or would you prefer me to follow up with a separate PR for that?

meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 21, 2026

facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label May 21, 2026

javache requested a review from zeyap May 21, 2026 10:43

zeyap reviewed May 21, 2026

View reviewed changes

zeyap requested changes May 21, 2026

View reviewed changes

jingjing2222 force-pushed the fix-android-fabric-sync-props-null branch 2 times, most recently from 8189b14 to 26a466a Compare May 21, 2026 14:32

jingjing2222 requested a review from zeyap May 21, 2026 14:33

jingjing2222 changed the title ~~Fix Android Fabric sync prop null override~~ Fix Android Fabric Native Animated prop reset May 21, 2026

zeyap reviewed May 21, 2026

View reviewed changes

jingjing2222 force-pushed the fix-android-fabric-sync-props-null branch from 26a466a to d25ff0d Compare May 21, 2026 15:18

Fix Android Fabric Native Animated prop reset

8fc40a7

jingjing2222 force-pushed the fix-android-fabric-sync-props-null branch from d25ff0d to 8fc40a7 Compare May 21, 2026 15:27

Scope Native Animated null override clearing

ccaa53e

jingjing2222 requested a review from zeyap May 21, 2026 16:05

Move Native Animated restore change out of PR

b9e2275

jingjing2222 changed the title ~~Fix Android Fabric Native Animated prop reset~~ Fix Android Fabric Native Animated null prop crash May 21, 2026

Use ASCII punctuation in sync mount props test

d21804f

Conversation

jingjing2222 commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this is correct

Changelog:

Test Plan:

Uh oh!

meta-codesync Bot commented May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeyap May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeyap May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeyap commented May 21, 2026

Uh oh!

jingjing2222 commented May 21, 2026

Uh oh!

zeyap May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeyap May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jingjing2222 commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jingjing2222 commented May 21, 2026 •

edited

Loading

zeyap May 21, 2026 •

edited

Loading

zeyap May 21, 2026 •

edited

Loading

zeyap May 21, 2026 •

edited

Loading

zeyap May 21, 2026 •

edited

Loading